Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added ignoring interfaces via -exclude_interfaces flag (#68) #72

Merged
merged 3 commits into from
Sep 15, 2023

Conversation

tulzke
Copy link
Contributor

@tulzke tulzke commented Aug 30, 2023

The problem is described in the issue. This is one of the solutions to this problem.

@CLAassistant
Copy link

CLAassistant commented Aug 30, 2023

CLA assistant check
All committers have signed the CLA.

@r-hang
Copy link
Contributor

r-hang commented Sep 6, 2023

Thanks for contribution @tulzke, i'm happy to discuss here or in the ticket but I think the the exclusion list as a mockgen flag parameter is the best approach as (1) command line flags are current mechanism by which mock options are configured and (2) users won't be required to modify their source code to take advantage of the feature.

@tulzke
Copy link
Contributor Author

tulzke commented Sep 7, 2023

@r-hang , thanks for reply. Yes, I can do this via the flag parameter.
What naming should I do for this? Maybe -exclude with comma separated interface names?
Example:
mockgen -exclude=Constraint,IgnoreMe -source=file.go -destination=file_mock.go

@tulzke tulzke changed the title added ignoring interfaces if they have a comment "gomock:ignore" (#68) added ignoring interfaces via -exclude flag (#68) Sep 7, 2023
@r-hang
Copy link
Contributor

r-hang commented Sep 7, 2023

@tulzke how about exclude_interfaces in case there is something in the future that we'd want to exclude from generated code.

@tulzke
Copy link
Contributor Author

tulzke commented Sep 8, 2023

@r-hang, sounds good. I have already committed this.

@r-hang
Copy link
Contributor

r-hang commented Sep 13, 2023

@tulzke the core logic looks could to me. However, could we add some tests for the newly added parseExcludeInterfaces function?

@tulzke
Copy link
Contributor Author

tulzke commented Sep 14, 2023

@r-hang good point. I added a unit test for the function, and then changed the implementation to satisfy the test cases.

@r-hang r-hang changed the title added ignoring interfaces via -exclude flag (#68) added ignoring interfaces via -exclude_interfaces flag (#68) Sep 15, 2023
@r-hang r-hang merged commit dad3840 into uber-go:main Sep 15, 2023
2 checks passed
sywhang pushed a commit that referenced this pull request Sep 25, 2023
This is a proposal PR to generate mocks without error for following
cases:
```go
type Water[R any, C UnsignedInteger] interface {
	Fish(R) []C
}

type UnsignedInteger interface {
	~uint | ~uint32 | ~uint64
}
```

Go `types` package seems to wrap interfaces that contain type
constraints by checking a type set literal of the form `~T` and `A|B`.
https://github.com/golang/go/blob/master/src/go/types/decl.go#L668

So gomock can just ignore that pattern to generate mocks safely without
`don't know how to mock method of type` errors. I think this can be a
better solution for custom type constraints than [`-exclude`
flag](#72).

---------

Co-authored-by: Jacob Oaks <[email protected]>
r-hang pushed a commit that referenced this pull request Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants